Conversation
…h tester för att ta emot meddelanden.
WalkthroughThe PR converts a simple JavaFX demo into a chat client: adds Ntfy messaging integration, a local HTTP image server, a stateful HelloModel, UI/controller changes for chat bubbles and image handling, new DTOs and connection implementations, tests and test doubles, updated module and Maven configuration, and new UI assets (FXML/CSS). Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as HelloController
participant Model as HelloModel
participant Conn as NtfyConnection
participant ImgServer as ImageServer
participant NtfyServer as Ntfy Server
rect rgb(200,220,255)
Note over User,UI: Sending a message/image
User->>UI: Enter text / choose image, click Send
UI->>Model: sendMessage(text) / sendImage(file)
alt image
Model->>ImgServer: POST /upload (file)
ImgServer-->>Model: 200 + http://localhost:8081/images/UUID.ext
end
Model->>Conn: send(JSON payload with message or image URL)
Conn->>NtfyServer: POST /<topic>
end
rect rgb(220,255,220)
Note over NtfyServer,Conn: Receiving messages (background)
Conn->>NtfyServer: GET /<topic>/json (poll/SSE)
NtfyServer-->>Conn: stream NtfyMessageDto events
Conn->>Model: callback with NtfyMessageDto
Model->>UI: ObservableList.add(message)
UI->>UI: render bubble or image, scroll to bottom
UI-->>User: message displayed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (12)
src/test/java/com/example/FxTestHelper.java (2)
4-6: Consider actual toolkit initialization if needed.The
initToolkit()method is currently a no-op. If JavaFX toolkit initialization is actually required for tests, consider usingPlatform.startup(() -> {})or similar. If no initialization is needed, the method name may be misleading.
5-5: Use English for code comments.The comment is in Swedish. For international collaboration and consistency, use English throughout the codebase.
Apply this diff:
- // Ingen GUI krävs för tester + // No GUI required for testssrc/main/java/com/example/HelloFX.java (1)
20-23: Improve error handling and formatting.Two issues: (1) Line 20 has inconsistent indentation with extra spaces, and (2)
printStackTrace()on line 22 is not suitable for production—use proper logging instead.Apply this diff:
new Thread(() -> { try { - new ImageServer(8081); - } catch (IOException e) { - e.printStackTrace(); - } + new ImageServer(8081); + } catch (IOException e) { + System.err.println("Failed to start ImageServer: " + e.getMessage()); + }src/main/java/com/example/ImageServer.java (1)
32-32: Avoid hardcoded localhost URL.The hardcoded
localhostURL on line 32 won't work when accessed from other machines. Consider making the hostname configurable or using the request'sHostheader.Example approach:
- String imageUrl = "http://localhost:" + port + "/images/" + filename; + String host = exchange.getRequestHeaders().getFirst("Host"); + String imageUrl = "http://" + (host != null ? host : "localhost:" + port) + "/images/" + filename;pom.xml (1)
64-69: Consider upgrading to WireMock 3.13.1 (stable release) or await 4.x stability.The latest stable version of WireMock is 3.13.1, whereas the project currently uses the beta
4.0.0-beta.15. Migrating to the stable 3.13.1 release eliminates beta stability risks, though it involves a major version downgrade from 4.x to 3.x. Alternatively, monitor the 4.x release cycle for a stable 4.x version to avoid regression in features or improvements.src/main/java/com/example/NtfyConnection.java (1)
6-23: ClarifysendImageandreceivecontracts in the interfaceThe abstraction looks good, but the semantics of
sendImage(File imageFile, String clientId)and the lifecycle ofreceive(Consumer<NtfyMessageDto> consumer)vsstopReceiving()are implicit. Consider adding brief Javadoc for:
- What
clientIdrepresents and whensendImagereturnsfalse.- Whether
receivemay be called multiple times and what happens if it is called afterstopReceiving().src/main/java/com/example/HelloController.java (2)
68-74: Extract"MartinsTopic"to a shared constantUsing the hard‑coded topic string
"MartinsTopic"here to determinesentByUsercouples the UI to a magic string that’s also used elsewhere (e.g., inNtfyConnectionSpyand tests). To avoid divergence, consider extracting this topic name into a shared constant (e.g., inHelloModelor a config class) and reusing it.
40-48: Prefer passing an owner window to theFileChooser
fileChooser.showOpenDialog(null)works but creates a dialog without a clear owner, which can cause focus/stacking quirks on some platforms. Consider passing the current window, for example:var window = messageField.getScene() != null ? messageField.getScene().getWindow() : null; File imageFile = fileChooser.showOpenDialog(window);src/test/java/com/example/HelloModelTest.java (1)
24-110: Good coverage of model and HTTP integration; consider tightening assertions laterThe tests nicely cover:
- Delegation to
NtfyConnectionvia the spy.- That incoming messages are stored in the model.
- Real HTTP interactions with
NtfyConnectionImplusing WireMock, including image upload flow.As a future enhancement, you might consider asserting full JSON payloads (instead of regex
matching(...)) to make the WireMock expectations more explicit, but this is optional given the current scope.src/main/java/com/example/mvnw.cmd (1)
1-189: Wrapper script looks standard, but the location is unconventionalThis is the stock Maven wrapper script and is fine as-is, but placing
mvnw.cmdundersrc/main/java/com/exampleis unusual — wrapper launchers are normally at the project root so developers can run./mvnwfrom there and tooling can detect them. Consider movingmvnw.cmd(and its companion files) to the repository root to align with typical Maven setups and avoid confusion.src/main/java/com/example/mvnw (1)
1-295: Unix mvnw script is fine, but should probably live at project rootThe script content matches the standard Maven Wrapper (3.3.4) and is OK, but having
src/main/java/com/example/mvnwinstead of a root-levelmvnwis non-standard and makes it harder to discover and use (./mvnwfrom project root won’t hit this file). I’d recommend relocating this file (and its.mvn/wrapperdirectory) to the repository root so Maven tooling and developers can use the wrapper in the usual way.src/main/java/com/example/HelloModel.java (1)
116-127: Tighten resource handling and error checks in uploadToLocalServer
uploadToLocalServerwrites the file and then reads the response bytes directly fromconn.getInputStream()without closing that stream explicitly or checking the HTTP status code. Whiledisconnect()will eventually clean up, a small adjustment makes it safer and clearer:- try (OutputStream os = conn.getOutputStream()) { - Files.copy(imageFile.toPath(), os); - } - String imageUrl = new String(conn.getInputStream().readAllBytes()); - conn.disconnect(); - return imageUrl; + try (OutputStream os = conn.getOutputStream()) { + Files.copy(imageFile.toPath(), os); + } + int status = conn.getResponseCode(); + if (status < 200 || status >= 300) { + throw new IOException("Upload failed with HTTP " + status); + } + try (InputStream in = conn.getInputStream()) { + String imageUrl = new String(in.readAllBytes()); + return imageUrl; + } finally { + conn.disconnect(); + }This ensures both streams are closed, and callers get a clear failure signal when the upload endpoint responds with an error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.gitignore(1 hunks)HelloFX.iml(1 hunks)pom.xml(4 hunks)src/main/java/com/example/.mvn/wrapper/maven-wrapper.properties(1 hunks)src/main/java/com/example/HelloController.java(1 hunks)src/main/java/com/example/HelloFX.java(1 hunks)src/main/java/com/example/HelloModel.java(1 hunks)src/main/java/com/example/ImageServer.java(1 hunks)src/main/java/com/example/NtfyConnection.java(1 hunks)src/main/java/com/example/NtfyConnectionImpl.java(1 hunks)src/main/java/com/example/NtfyMessageDto.java(1 hunks)src/main/java/com/example/mvnw(1 hunks)src/main/java/com/example/mvnw.cmd(1 hunks)src/main/java/module-info.java(1 hunks)src/main/resources/com/example/chat-style.css(1 hunks)src/main/resources/com/example/hello-view.fxml(1 hunks)src/test/java/com/example/FxTestHelper.java(1 hunks)src/test/java/com/example/HelloModelTest.java(1 hunks)src/test/java/com/example/NtfyConnectionSpy.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/module-info.java (1)
src/main/java/com/example/HelloFX.java (1)
HelloFX(12-43)
src/test/java/com/example/HelloModelTest.java (1)
src/test/java/com/example/FxTestHelper.java (1)
FxTestHelper(3-7)
🪛 Biome (2.1.2)
src/main/resources/com/example/chat-style.css
[error] 2-2: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 3-3: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 7-7: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 8-8: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 9-9: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 10-10: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 11-11: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 12-12: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 13-13: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 14-14: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 17-17: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 17-17: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 18-18: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 23-23: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 24-24: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 25-25: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 26-26: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 27-27: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 28-28: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 29-29: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 30-30: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 34-34: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 38-38: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
🔇 Additional comments (7)
.gitignore (1)
3-3: LGTM!Adding
.envto.gitignoreis a best practice to prevent committing environment variables and secrets to version control.HelloFX.iml (1)
1-9: LGTM!Standard IntelliJ IDEA module configuration with appropriate source folder declarations.
src/main/java/com/example/.mvn/wrapper/maven-wrapper.properties (1)
1-3: LGTM!Standard Maven Wrapper configuration with appropriate distribution settings.
src/main/resources/com/example/chat-style.css (1)
1-39: LGTM! Static analysis warnings are false positives.The static analysis tool (Biome) is flagging JavaFX-specific CSS properties as unknown. This is expected—JavaFX CSS uses the
-fx-prefix and includes properties like-fx-background-color,-fx-background-radius, etc., which are valid in JavaFX but not standard CSS. The stylesheet is correctly formatted for JavaFX.pom.xml (1)
50-63: I need to verify jackson-annotations as well and confirm these are the latest versions. Let me search for additional information.Based on the verification results:
No current security vulnerabilities found, but consider updating jackson libraries to the latest 2.x versions.
jackson-core 2.17.2 has no direct vulnerabilities, and the security advisory check confirmed no matching vulnerabilities for jackson-databind 2.17.2 (all reported CVEs affect older versions like 2.9.x, 2.12.x, and 2.13.x ranges). dotenv-java 3.2.0 also shows no known security advisories.
However, the latest version of jackson-databind is 2.20.1, and your project uses 2.17.2—four minor versions behind. Jackson core components (annotations, streaming, databind) must have the same minor version, so consider upgrading both jackson-databind and jackson-annotations from 2.17.2 to 2.20.1 to stay current with the latest patches and improvements.
src/main/java/module-info.java (1)
1-13: Module dependencies look consistent; verify module name usageThe added
requiresand theopens com.example to javafx.fxml, com.fasterxml.jackson.databind;line align with JavaFX + Jackson usage. Please just double‑check that:
- The module name
HelloFXmatches your build/runtime configuration (e.g.,moduleoption, IDE run config).- No old references to a previous module name remain (if it was renamed).
src/main/resources/com/example/hello-view.fxml (1)
10-31: FXML wiring matches the controller and looks consistentThe ids (
messageField,chatBox,chatScroll,statusLabel) and action handlers (#handleSend,#handleSendImage) line up withHelloController, and the layout is clean for a chat UI.
| server.createContext("/upload", exchange -> { | ||
| if ("POST".equals(exchange.getRequestMethod())) { | ||
| String filename = UUID.randomUUID() + ".jpg"; | ||
| Path filePath = imageDir.resolve(filename); | ||
| Files.copy(exchange.getRequestBody(), filePath, StandardCopyOption.REPLACE_EXISTING); | ||
|
|
||
| String imageUrl = "http://localhost:" + port + "/images/" + filename; | ||
| byte[] response = imageUrl.getBytes(); | ||
| exchange.sendResponseHeaders(200, response.length); | ||
| exchange.getResponseBody().write(response); | ||
| } | ||
| exchange.close(); | ||
| }); |
There was a problem hiding this comment.
Add upload validation and size limits.
The upload endpoint has several security and quality issues:
- No file size limit—an attacker could fill disk space
- No content type validation—any data is accepted as an image
- All files saved as
.jpgregardless of actual format (line 28) - No authentication/authorization
Consider implementing:
- Maximum file size check before accepting upload
- Content type validation (check headers and/or magic bytes)
- Determine actual file extension from content type
- Basic authentication or token-based authorization
Ensure exchange is closed in all cases.
The exchange.close() on line 37 should be in a finally block to ensure it executes even if an exception occurs during request processing.
Apply this diff:
server.createContext("/upload", exchange -> {
- if ("POST".equals(exchange.getRequestMethod())) {
- String filename = UUID.randomUUID() + ".jpg";
- Path filePath = imageDir.resolve(filename);
- Files.copy(exchange.getRequestBody(), filePath, StandardCopyOption.REPLACE_EXISTING);
-
- String imageUrl = "http://localhost:" + port + "/images/" + filename;
- byte[] response = imageUrl.getBytes();
- exchange.sendResponseHeaders(200, response.length);
- exchange.getResponseBody().write(response);
+ try {
+ if ("POST".equals(exchange.getRequestMethod())) {
+ String filename = UUID.randomUUID() + ".jpg";
+ Path filePath = imageDir.resolve(filename);
+ Files.copy(exchange.getRequestBody(), filePath, StandardCopyOption.REPLACE_EXISTING);
+
+ String imageUrl = "http://localhost:" + port + "/images/" + filename;
+ byte[] response = imageUrl.getBytes();
+ exchange.sendResponseHeaders(200, response.length);
+ exchange.getResponseBody().write(response);
+ }
+ } finally {
+ exchange.close();
}
- exchange.close();Apply the same pattern to the /images endpoint (lines 41-50).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| server.createContext("/upload", exchange -> { | |
| if ("POST".equals(exchange.getRequestMethod())) { | |
| String filename = UUID.randomUUID() + ".jpg"; | |
| Path filePath = imageDir.resolve(filename); | |
| Files.copy(exchange.getRequestBody(), filePath, StandardCopyOption.REPLACE_EXISTING); | |
| String imageUrl = "http://localhost:" + port + "/images/" + filename; | |
| byte[] response = imageUrl.getBytes(); | |
| exchange.sendResponseHeaders(200, response.length); | |
| exchange.getResponseBody().write(response); | |
| } | |
| exchange.close(); | |
| }); | |
| server.createContext("/upload", exchange -> { | |
| try { | |
| if ("POST".equals(exchange.getRequestMethod())) { | |
| String filename = UUID.randomUUID() + ".jpg"; | |
| Path filePath = imageDir.resolve(filename); | |
| Files.copy(exchange.getRequestBody(), filePath, StandardCopyOption.REPLACE_EXISTING); | |
| String imageUrl = "http://localhost:" + port + "/images/" + filename; | |
| byte[] response = imageUrl.getBytes(); | |
| exchange.sendResponseHeaders(200, response.length); | |
| exchange.getResponseBody().write(response); | |
| } | |
| } finally { | |
| exchange.close(); | |
| } | |
| }); |
…, rätt stängning av stream, felmeddelande om upload misslyckas.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/com/example/ImageServer.java (1)
34-57: Upload endpoint still lacks size/type validation—consider hardening it.The
/uploadhandler still accepts arbitrary bytes with no file size limit or content‑type check. For anything beyond a trusted local lab setup, you should add basic validation (e.g., reject bodies over some MB threshold, checkContent-Typeand/or magic bytes, and avoid always forcing the.jpgextension) to reduce abuse and disk‑fill risk.
🧹 Nitpick comments (5)
src/test/java/com/example/NtfyConnectionSpy.java (3)
24-29: Consider making the return value configurable for failure testing.The method always returns
true, preventing tests from verifying error-handling paths inHelloModel.sendImage(). For more comprehensive testing, consider adding a field likeboolean simulateImageUploadFailureto toggle the return value.Example enhancement:
public class NtfyConnectionSpy implements NtfyConnection { public String lastSentMessage; public File lastSentImage; public String lastClientId; public Consumer<NtfyMessageDto> messageHandler; + public boolean simulateImageUploadFailure = false; @Override public boolean sendImage(File imageFile, String clientId) { this.lastSentImage = imageFile; this.lastClientId = clientId; System.out.println("🧪 Spy sendImage(): " + imageFile.getName() + " | clientId=" + clientId); - return true; + return !simulateImageUploadFailure; }
37-38: Consider clearing the handler and adding debug output.The empty
stopReceiving()implementation leavesmessageHandleractive, allowingsimulateIncomingMessage()to deliver messages even after stopping. For clearer test semantics, consider clearing the handler.Apply this diff:
@Override -public void stopReceiving() { } +public void stopReceiving() { + this.messageHandler = null; + System.out.println("🧪 Spy stopReceiving() called"); +}
43-58: Good fix on time units; consider adding null safety.Line 45 correctly uses
System.currentTimeMillis() / 1000to provide epoch seconds, resolving the past review concern.However, the method lacks a null check on the
jsonparameter, which could cause aNullPointerExceptionon line 52 or 54.Apply this diff to add null safety:
public void simulateIncomingMessage(String json) { + if (json == null) { + throw new IllegalArgumentException("json cannot be null"); + } if (messageHandler != null) { long now = System.currentTimeMillis() / 1000;Note: The regex-based extraction remains acceptable for simple test payloads, as previously discussed.
src/main/java/com/example/HelloModel.java (1)
129-149: Consider adding HTTP timeouts foruploadToLocalServerto avoid indefinite blocking.Right now the upload uses a raw
HttpURLConnectionwithout connect/read timeouts, so a hung or unreachable image server can block the caller indefinitely. Adding reasonable timeouts would make failures predictable:- HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - conn.setDoOutput(true); - conn.setRequestMethod("POST"); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + conn.setDoOutput(true); + conn.setRequestMethod("POST"); + conn.setConnectTimeout(5_000); + conn.setReadTimeout(10_000);src/main/java/com/example/HelloController.java (1)
42-58: Verify thattopicis a reliable indicator of “sent by user”.
sentByUseris computed asmsg.topic().equals(HelloModel.DEFAULT_TOPIC). If all chat participants publish to the same Ntfy topic, remote messages will likely have the same topic and be rendered as if they were sent by this client (right‑aligned). Consider basing this on a per‑client identifier (e.g.clientIdembedded in the payload) instead, or otherwise confirm that your connection layer sets distinct topics for local vs remote messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
pom.xml(3 hunks)src/main/java/com/example/HelloController.java(1 hunks)src/main/java/com/example/HelloFX.java(1 hunks)src/main/java/com/example/HelloModel.java(1 hunks)src/main/java/com/example/ImageServer.java(1 hunks)src/main/java/com/example/NtfyConnection.java(1 hunks)src/main/java/com/example/NtfyConnectionImpl.java(1 hunks)src/main/resources/com/example/chat-style.css(1 hunks)src/test/java/com/example/HelloModelTest.java(1 hunks)src/test/java/com/example/NtfyConnectionSpy.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/example/NtfyConnection.java
- src/test/java/com/example/HelloModelTest.java
- src/main/java/com/example/NtfyConnectionImpl.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/com/example/NtfyConnectionSpy.java (1)
src/main/java/com/example/HelloModel.java (1)
HelloModel(19-153)
src/main/java/com/example/HelloController.java (1)
src/main/java/com/example/HelloModel.java (1)
HelloModel(19-153)
🪛 Biome (2.1.2)
src/main/resources/com/example/chat-style.css
[error] 2-2: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 3-3: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 7-7: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 8-8: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 9-9: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 10-10: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 11-11: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 12-12: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 13-13: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 14-14: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 18-18: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 19-19: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 20-20: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 21-21: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 22-22: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 23-23: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 24-24: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 25-25: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 29-29: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 30-30: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 34-34: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 35-35: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 40-40: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 40-40: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 41-41: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 42-42: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 43-43: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 44-44: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 48-48: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 49-49: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 53-53: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 57-57: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 61-61: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 62-62: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 63-63: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
🔇 Additional comments (8)
src/main/resources/com/example/chat-style.css (1)
1-64: Well-designed chat UI stylesheet; static analysis flags are false positives.This CSS file provides solid styling for the chat application with a cohesive color scheme (green for sent messages, white/gray for received), appropriate contrast, and well-structured layout. All properties use valid JavaFX CSS syntax with the
-fx-prefix.The 36 errors from Biome are false positives—Biome is a web CSS linter and doesn't recognize JavaFX-specific CSS properties and functions like
-fx-effectwithdropshadow(). These are all valid JavaFX CSS and will work correctly.src/test/java/com/example/NtfyConnectionSpy.java (3)
10-15: LGTM! Appropriate test spy pattern.The public mutable fields are acceptable for test doubles, allowing tests to inspect recorded values and verify behavior.
17-21: LGTM! Clean spy implementation.The method correctly records the sent message for test verification and includes helpful debug output.
31-35: LGTM! Handler registration works correctly.The method properly stores the message handler for use in
simulateIncomingMessage().pom.xml (1)
8-18: POM dependency and plugin setup looks consistent; just verify external versions.The earlier JUnit and
javafx-swingversion inconsistencies are now resolved, and the new deps/plugins (JavaFX controls/FXML/Swing, dotenv, Jackson, WireMock, surefire withuseModulePath=false) look coherent for this module-based JavaFX app. Please just double‑check that the chosen versions (notablyjavafx.version=25and the Jackson/WireMock versions) exist in your target repo and work with your JDK/toolchain.Also applies to: 21-76, 78-108
src/main/java/com/example/HelloModel.java (1)
20-27: Messaging flow and FX-thread safety look solid now.The model wiring to
NtfyConnection, the Markdown image detection using substring/indexes, and the updatedaddMessageSafelythat marshals list updates onto the FX Application Thread together address the earlier regex and threading concerns. The ObservableList should now be safe to bind/listen to from the UI.Also applies to: 47-77, 84-127
src/main/java/com/example/HelloFX.java (1)
12-61: Single connection + ImageServer lifecycle wiring looks good.Using one
NtfyConnectionImplinstance injected intoHelloController, startingImageServeron a daemon thread, and stopping both in thestage.setOnCloseRequesthandler provides a clean lifecycle and resolves the earlier duplicate‑connection/shutdown concerns.src/main/java/com/example/ImageServer.java (1)
17-23: HTTP server structure, path normalization, and exchange closing look correct.
imageDiris normalized, both/uploadand/imageshandlers validate that resolved paths stay under this directory, andexchange.close()is now infinallyblocks. This addresses the earlier path traversal and resource‑cleanup concerns and gives you a robust baseline image server for the chat.Also applies to: 28-69, 71-108, 115-120
| /** | ||
| * Test-double for HelloModel. | ||
| * Sends and receive messages without actual HTTP. | ||
| */ |
There was a problem hiding this comment.
Minor typo in documentation.
Line 8 has "Sends and receive messages" but should be "Sends and receives messages" for grammatical correctness.
Apply this diff:
/**
* Test-double for HelloModel.
- * Sends and receive messages without actual HTTP.
+ * Sends and receives messages without actual HTTP.
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Test-double for HelloModel. | |
| * Sends and receive messages without actual HTTP. | |
| */ | |
| /** | |
| * Test-double for HelloModel. | |
| * Sends and receives messages without actual HTTP. | |
| */ |
🤖 Prompt for AI Agents
In src/test/java/com/example/NtfyConnectionSpy.java around lines 6 to 9, the
Javadoc comment has a minor grammatical typo: change "Sends and receive messages
without actual HTTP." to "Sends and receives messages without actual HTTP." so
the verb agrees with the subject; update the comment accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/example/HelloController.java (1)
43-61: Platform.runLater wrapper still present around Change iteration.Past review comments correctly identified that
ListChangeListener.Changeshould be processed synchronously within the callback, not wrapped inPlatform.runLater(), as the Change object is only valid during the notification. However, sinceHelloModel.addMessageSafely()(lines 140-148 in HelloModel.java) already marshals updates to the FX thread when needed, the listener should execute on the FX thread. Remove thePlatform.runLater()wrapper and process the change directly.
🧹 Nitpick comments (2)
src/main/java/com/example/HelloModel.java (1)
81-105: Consider using a logger instead of printStackTrace.The image sending logic is solid, but
printStackTrace()at line 102 should ideally be replaced with proper logging (e.g.,java.util.logging.Loggeror SLF4J) for production code.src/main/java/com/example/NtfyConnectionImpl.java (1)
29-42: Consider proper logging framework.The send implementation is correct, but
printStackTrace()at line 40 should be replaced with a proper logging framework for better observability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/example/HelloController.java(1 hunks)src/main/java/com/example/HelloFX.java(1 hunks)src/main/java/com/example/HelloModel.java(1 hunks)src/main/java/com/example/ImageServer.java(1 hunks)src/main/java/com/example/NtfyConnectionImpl.java(1 hunks)src/main/java/com/example/NtfyMessageDto.java(1 hunks)src/test/java/com/example/HelloModelTest.java(1 hunks)src/test/java/com/example/NtfyConnectionSpy.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/example/ImageServer.java
- src/test/java/com/example/HelloModelTest.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/com/example/NtfyConnectionSpy.java (1)
src/main/java/com/example/HelloModel.java (1)
HelloModel(19-185)
src/main/java/com/example/NtfyConnectionImpl.java (1)
src/main/java/com/example/HelloModel.java (1)
HelloModel(19-185)
🔇 Additional comments (21)
src/main/java/com/example/NtfyMessageDto.java (1)
1-24: LGTM! Clean DTO structure.The use of Java records for immutable DTOs is excellent, and the Jackson annotations are properly applied to handle unknown JSON fields gracefully. The structure aligns well with the Ntfy message format.
src/main/java/com/example/HelloFX.java (4)
19-26: Good error handling for missing configuration.The dotenv validation with early return prevents the application from starting with invalid configuration, providing clear feedback via
showError().
28-37: Proper resource management and initialization.Both the connection and image server are correctly stored in instance fields and will be properly cleaned up in the shutdown hook. The directory creation and error handling are appropriate.
48-49: Connection injection properly implemented.The controller now correctly receives the connection instance created in
HelloFX, ensuring a single shared connection is used throughout the application lifecycle.
55-69: Proper resource cleanup on application shutdown.The shutdown hook correctly stops both the connection and image server, with independent error handling to ensure one failure doesn't prevent the other from being cleaned up.
src/test/java/com/example/NtfyConnectionSpy.java (2)
10-39: Clean test double implementation.The spy correctly implements all
NtfyConnectionmethods with simple tracking behavior suitable for testing. The public fields enable test verification without complex assertion APIs.
44-59: Correct time unit conversion for test messages.The timestamp is properly converted from milliseconds to seconds (line 46), ensuring consistency with production code expectations.
src/main/java/com/example/HelloModel.java (5)
19-40: Well-structured model with proper validation.The dual constructor approach cleanly separates GUI and headless modes for testing, and the null check prevents invalid construction.
61-76: Proper message sending with optimistic UI update.The implementation correctly validates input, sends the message, and immediately updates the local message list for responsive UI feedback. Time conversion to seconds is correct.
110-134: Proper message filtering and handling.The clientId-based filtering correctly prevents echo, and the separate handling paths for image vs. text messages ensure proper UI rendering.
140-148: Thread-safe ObservableList updates.The implementation correctly ensures all UI updates happen on the JavaFX Application Thread while supporting headless testing.
153-183: Solid HTTP upload implementation with proper validation.The MIME type check, timeouts, and resource management are well-implemented. The hardcoded
localhost:8081URL is acceptable for this local image server scenario.src/main/java/com/example/NtfyConnectionImpl.java (4)
13-26: Proper initialization with validation.The constructor correctly validates the hostname and initializes thread-safe resources. The
volatileflag ensures visibility across threads.
45-47: Clear indication of moved functionality.The explicit exception with a descriptive message clearly communicates that image sending is now handled at the
HelloModellayer.
50-84: Polling-based message reception with proper lifecycle management.The implementation uses polling rather than long-lived HTTP streaming. The daemon thread, error handling with backoff, and proper interrupt handling are all correctly implemented. The 1-second poll interval is reasonable for a chat application.
87-92: Clean thread termination.The
stopReceiving()method correctly signals termination and interrupts the receiver thread, ensuring clean shutdown.src/main/java/com/example/HelloController.java (5)
32-41: Flexible injection supporting both model and connection.The dual injection methods support different initialization paths while ensuring
attachListeners()is always called.
65-71: Good UX with Enter-to-send and auto-scroll.The keyboard shortcut and automatic scrolling to show new messages are user-friendly touches.
74-82: Standard message send with proper cleanup.The implementation correctly validates, sends, and clears the input field.
85-108: Robust file selection with validation and feedback.The FileChooser configuration, file existence check, and status feedback provide good error handling and user experience.
110-171: Well-implemented message rendering with good UX.The bubble styling, timestamp formatting, and image enlargement feature create a polished chat interface. The consistent use of
Instant.ofEpochSecond()correctly expects timestamps in seconds.
2 participants
Summary by CodeRabbit
New Features
Tests